Autodiff #40
Autodiff #40
Conversation
|
I forgot to mention, the new method for |
|
As you can see in the above plots, it's accurate to within 1%. (This is also true for larger values of x.) At some point, someone who is better at math than me could probably get something working which is more compact/accurate, but I think this is definitely good enough for our purposes. |
|
I updated this branch with rebase/force-push. I think everything worked but if there's git problems that's probably why. |
…in vald linelists
|
This is a really ugly combo of different things, but it would be great to get it merged. Everything except the "early fitting code" is ready for review. What do you think is the best way to go about it? |
Codecov Report
@@ Coverage Diff @@
## main #40 +/- ##
==========================================
- Coverage 78.94% 69.41% -9.53%
==========================================
Files 13 14 +1
Lines 532 618 +86
==========================================
+ Hits 420 429 +9
- Misses 112 189 +77
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This seems good to me.
I had a few minor comments.
I also suggested a bunch of changes to the continuum opacity function signatures that make check to make sure that the individual arguments receive arguments of the same type. This is a lot closer to what I originally had and is consistent with the style of function signatures that you use in line_opacity.jl and 'line_list.jl`. I also think that this style makes the return type a lot more predictable and helps avoid bugs. But if there's a reason that we can't do this, just let me know. (Or if there is reason you don't want to do this)
Co-authored-by: Matthew Abruzzo <[email protected]>
Co-authored-by: Matthew Abruzzo <[email protected]>
Co-authored-by: Matthew Abruzzo <[email protected]>
|
Not that I would expect you to re-review and merge this today, but please don't merge. I think I've identified a bug and am working to fix it. |
|
No bug per se, but I think the "excess broadening" we've been seeing is actually continuum opacities that are too small, presumably because I've had to comment out various sources that don't have good data in the IR. |
This makes types generic enough to support autodiff. Unfortunately, it is also necessary to define a method of
floatmaxwhich is defined on the relevantForwardDiff.Dualsubtype, e.g.where
synis the function being differentiated, e.g. the spectrum as a function of Teff and vmic:I will make sure this is documented at some point, and will take care of it myself in the Korg parameter estimation routines.
BTW, calculating the Jacobian in this example takes the same amount of time as synthesizing the spectrum. Not bad!
P.S. this is branched off #32. I will fix the diff before review.